Enhance dependency management and update README for optional dependen… #23
Conversation
…cies - Added new optional dependencies in `pyproject.toml`, including `pydash` and `fastapi[standard]`. - Updated the README to reflect changes in optional dependencies and their usage. - Introduced a new test script to validate the functionality of `UserAttributeUpdater` with only base dependencies. - Refactored the CLI to handle missing optional dependencies gracefully, providing informative error messages. - Improved logging configuration to ensure compatibility with missing libraries.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @bwebs, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request significantly improves the project's dependency management strategy by making more components optional, thereby reducing the default installation size. It enhances the user experience by providing graceful error handling for CLI commands when optional dependencies are missing and ensures core features remain functional with minimal installations. Additionally, it refines logging to be more resilient and updates documentation to reflect these changes.
Highlights
- Refactored Dependency Management: Moved several core CLI dependencies (like
typer,rich,structlog,questionary,requests,cryptography) from base dependencies to a newclioptional dependency group inpyproject.toml. This reduces the base installation footprint and allows users to install only the components they need. - Graceful CLI Degradation: Implemented dynamic loading for optional CLI command groups (
mcp,observability,tools). If a user attempts to run a command whose required optional dependencies are not installed, the CLI will now provide an informative error message and suggest installing the relevant extra, rather than crashing. - Enhanced Logging Robustness: Updated logging configurations to conditionally import and use
structlogandrichcomponents. This ensures the application's logging system remains functional even if these optional libraries are not present, preventingModuleNotFoundErrorexceptions. - Updated Documentation: The
README.mdhas been revised to reflect the new optional dependency structure, including renamingembed-observabilitytoobservabilityanduser-attribute-updatertotools, and simplifying the descriptions of available extras. - Validated Base Functionality: Introduced a new test script and a dedicated test case to verify that core functionalities, specifically the
UserAttributeUpdater, can be imported and used successfully when only the project's base dependencies are installed, without any optional extras.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
The pull request effectively refactors dependency management by introducing optional dependencies and updating the CLI to handle missing libraries gracefully. The logging configuration has also been improved for compatibility. However, there are a few areas that require attention regarding dependency consistency and test correctness.
| if not structured_logger: | ||
| raise Exception("Structured logger is not available") |
There was a problem hiding this comment.
The structured_logger is part of the cli optional dependency, not the observability extra. This check introduces a hard dependency on structlog for the observability module, which contradicts the goal of handling missing optional dependencies gracefully. If structured_logger is truly required for this module, it should be explicitly listed as a dependency for the observability extra in pyproject.toml. Otherwise, consider falling back to the base logger or handling its absence more gracefully to prevent a hard crash.
if not structured_logger:
# Fallback to base logger or handle gracefully if structlog is truly optional
# For now, raising an exception as it's a critical component for this module.
raise Exception("Structured logger is not available")| assert '"value":"test"' not in output | ||
| assert '"update_type":"default"' in output |
There was a problem hiding this comment.
The assertion assert '"value":"test"' not in output appears to be incorrect. The UserAttributeUpdater class (from lkr/tools/classes.py) defines value as a standard field. By default, model_dump_json() should include all fields, so value is expected to be present in the JSON output. This assertion will likely cause the test to fail if model_dump_json() behaves as expected. Please verify if value is indeed intended to be excluded from the JSON output, and if so, adjust the UserAttributeUpdater's Pydantic configuration (e.g., using Field(exclude=True)) or modify the test assertion.
| assert '"value":"test"' not in output | |
| assert '"update_type":"default"' in output | |
| assert '"user_attribute":"test"' in output | |
| assert '"value":"test"' in output | |
| assert '"update_type":"default"' in output |
There was a problem hiding this comment.
this is correct, value is listed as exclude
lkr/main.py
Outdated
| logger.error(msg) | ||
| logger.error(import_error) | ||
| raise typer.Exit(0) |
There was a problem hiding this comment.
When an error occurs, it's best practice to exit with a non-zero status code (e.g., typer.Exit(1)) to indicate failure. Exiting with 0 typically signifies successful execution, which might be misleading for a command that failed to load due to missing dependencies.
| logger.error(msg) | |
| logger.error(import_error) | |
| raise typer.Exit(0) | |
| logger.error(msg) | |
| logger.error(import_error) | |
| raise typer.Exit(1) |
…cies
pyproject.toml, includingpydashandfastapi[standard].UserAttributeUpdaterwith only base dependencies.